Skip to content

feat(rum): let the console set the session sample rate - #30

Open
Fiona2016 wants to merge 27 commits into
publishfrom
feat/remote-sampling-configuration
Open

feat(rum): let the console set the session sample rate#30
Fiona2016 wants to merge 27 commits into
publishfrom
feat/remote-sampling-configuration

Conversation

@Fiona2016

Copy link
Copy Markdown
Collaborator

Lets an application owner change how much traffic RUM keeps without the customer
shipping a new release.

Off by default: without setRemoteConfigurationEnabled(true) the SDK makes no
extra request and behaves exactly as before.

How it works

  • Fetching follows the rhythm of the sessions that read it: once at start-up and
    once whenever a new session begins. There is no timer between sessions — a
    change can only matter at the next draw.
  • Values persisted by a previous launch apply before the network answers, so the
    first sessions after a cold start already draw with what the console last
    provided rather than with the values the app was built with.
  • A failed, timed-out or unreadable response leaves the stored values exactly as
    they were, and is retried twice (5s then 60s, jittered) before waiting for the
    next natural trigger.
  • The stored configuration is revalidated with If-None-Match, so an unchanged
    answer costs a 304 rather than a body.
  • An operator can additionally allow a refresh when the app returns to the
    foreground, gated by staleness. It is off unless turned on for the application:
    session starts spread requests across the day, while returning to the
    foreground bunches them at the moment everyone opens the app.
  • Each session records the rate it was actually drawn under and the configuration
    version it came from; its events report both, so server-side extrapolation
    lines up with the draw that kept the session.
  • RumMonitor.getRemoteConfig() returns the console's custom values, decoded.
    Nested objects and arrays come back as Map and List; a body that cannot be
    read answers as nothing published, since no rate or decision depends on it.
  • RumMonitor.setForcedSession() collects the current visitor regardless of the
    rates, for a support or debugging flow.

Only the session sample rate is delivered. Session Replay and distributed tracing
are not configured from here on this platform, so the replay sample rate stays
where the app configures it and Session Replay draws with exactly the sampler the
app was built with.

Verification

  • :features:dd-sdk-android-rum:testDebugUnitTest green
  • API surface regenerated; the only public change is getRemoteConfig() and
    setForcedSession() on RumMonitor plus the builder switch

Both sampling rates were fixed when the app called `RUM.enable()`, so
changing either one meant shipping a new release and waiting for users to
update. That is days or weeks at exactly the moments the knob is worth
having: an incident, a launch, a bill that jumped overnight.

With `setRemoteConfigurationEnabled(true)` the SDK takes the session and
session replay sample rates from the application's settings instead,
polling `/api/v2/rum/config` for them. Left off — the default — nothing is
requested and the SDK behaves exactly as before.

The rates are read where a session's fate is decided, in `renewSession`, so
a change never disturbs a session already under way: it applies from the
next one. The server can also ask for immediate activation, in which case
the session is restarted as soon as rates that actually change this client
arrive, so a new one starts under them. Restarting rather than flipping the
running session in place keeps every session a complete record of itself.

The replay rate travels to Session Replay on the message RUM already sends
it when a session is renewed, so one request drives both decisions and
there is no second store to keep in step.

Failure is always "keep collecting with what you have": nothing here can
delay initialisation, an error or timeout leaves the stored rates
untouched, and a rate the server does not send stays with the value passed
at init — a rate is never invented, least of all a zero, which would switch
off collection nobody asked to switch off.

Events keep reporting the rate their session was really drawn at rather
than the one the app was built with, so the configured sample rate on an
event stays true.
An app spends most of its life in the background, where the poll timer
cannot be trusted: the system may not run it for hours. Someone could
reopen the app and carry on under settings that were changed while it was
away.

Returning to the foreground is now its own reason to ask, subject to the
same ttl, so switching between apps does not turn into a request each
time. Rotations and activity-to-activity navigation keep the started count
above zero, so neither is mistaken for a return.

Deliberately not a method the app has to call: the apps that would never
get fresh settings are exactly the ones that never read far enough to find
such a method.

The ttl the server asked for is now remembered when the response is read
rather than around the request, so a fetch that fails keeps it instead of
falling back to ours.
The console had no honest way to tell whether a saved change had reached
anyone. Events cannot answer it: an event only exists for a session that
was kept, so at a low sample rate they describe the sampled few, and the
size of that blind spot is set by the very rate being changed.

The version each response carried is now stored alongside the rates and
sent back on the next request — the one request every client makes,
whether or not its session was kept. It is kept even when the response
carried no rates, which is what 'remote configuration is off, use your own
settings' looks like, so the console can still see the app is up to date
with the change that turned them off.
Asking again when the app returned to the foreground was unconditional.
The poll spreads requests across the ttl; returning to the foreground does
the opposite, bunching them at the moment everyone opens the app — the same
shape as a release herd, and the ttl throttle bounds the rate rather than
the shape.

It now happens only when the configuration says so, which is off by
default.
setForcedSession() on RumMonitor is the escape hatch for "collect this
user now": the application knows who needs debugging (its own
allow-list, a support flow), the SDK only provides the switch. The
session restarts so the forced draw applies from a clean session - RUM
cannot flip the replay decision of one already under way - and the
renewal message tells Session Replay to skip its own draw, so a forced
session always comes out with replay. Calling again while the forced
session runs is a no-op, and the forced state lasts for the process
lifetime, so the application decides on each app start whether to call
again.
The console can publish a small bag of application-defined JSON values
alongside the sampling settings; the SDK persists it with them and
hands it to the host application verbatim through
RumMonitor.getRemoteConfig(), as the raw JSON object string, never
interpreting it. What a value means is entirely up to the application's
own code - a debug allow-list to pair with setForcedSession(), a
feature toggle. The bag is persisted like the rates, so what one launch
fetched answers immediately on the next; when the kill switch turns
remote configuration off, the bag goes with it. A custom-only change
never restarts a session - immediate activation keeps comparing rates
alone.
…tion

The channel no longer carries only sampling rates - the console's
contract grew a trace sample rate and a replay privacy level - so
everything internal that called it "sampling" takes the broader name:
the controller, the store and its preferences file, and every symbol
wired through the feature, the monitor and the scopes. The public init
option is untouched (setRemoteConfigurationEnabled already said it),
as is the endpoint and every behaviour; this commit only moves names.
The ttl poll timer is gone. A fetch now happens at start-up and after
every session draw - the one rhythm a change can matter on, since the
draw for the running session has already happened and the response
lands in storage for the next one, which is exactly the next-session
semantics the console promises. The server's ttl field stays, but only
bounds staleness for the (server-gated) foreground refresh; a polling
mode may come back later and the protocol field is reserved for it.

A failed fetch is retried quickly (5s), then patiently (60s), then not
at all until the next natural trigger: two extra requests per outage
per client, so a fleet can never turn an endpoint incident into a
storm. Each delay is spread by +/-20% so recovering clients do not all
return at the same moment. A new trigger cancels a waiting retry and
re-arms the backoff, and a failure never clears the stored values.

Wiring note: sessions created by startNewSession (after the first) now
also receive the remote configuration store, which the earlier wiring
had only given to the very first session scope.
The server answers a conditional request with 304 when nothing
changed, so the SDK now stores the ETag beside the configuration it
validated and echoes it back as If-None-Match. The validator belongs
to that stored answer specifically - the body varies per caller
context - so it lives in the same store and is kept even by the kill
switch, whose answer is what the next revalidation stands on. A 304
counts as a success: nothing to apply, no retry owed, and the
staleness bookkeeping moves on.

The store key now covers everything that can change the answer: the
storage format version (a prefix, bumped on format change rather than
on SDK upgrade), the endpoint host, the RUM application id, the
service, the environment and the app version. It deliberately still
leaves out the SDK version, which would throw the cache away on every
upgrade. The SDK version goes on the request instead, as sdk_version,
for the server's future targeting.

Store persistence gains its first unit tests (round-trip across
instances, omitted knobs forgotten, kill switch keeps the version,
storage unavailable falls back to init) via an in-memory
SharedPreferences.
…events

Events used to carry the init sampling rate even when the console's
settings decided the draw, skewing server-side extrapolation. Each
session draw now records a DrawnConfiguration - the rates it actually
used (the console's where it set them, the init values where it did
not) and the remote settings version they came from - married to the
session id and kept in storage next to the settings cache, so a stale
record is inert rather than wrong. View events report the drawn rates
in _dd.configuration, now also populating the existing
session_replay_sample_rate field, and carry rc_version naming the
settings version so an audit can recover the exact configuration from
the version history (0 when none was ever fetched; the field is a
FlashCat addition to the view schema - our intake reads it, others
ignore it).

The drawn replay rate falls back to what Session Replay publishes
about its own configuration, since the console-side rate lives on the
RUM feature. Sessions drawn without remote configuration report
nothing new - for them the init values are the drawn values.

Also covers the previously untested remote read at session renewal and
the replay rate riding the session-renewed bus message.
The field landed with the event change; the generated surface files did not go
with it, so the api-surface check would have failed on the next run for a change
that was already made.

Only the RUM surface. The session-replay-noop surface is also stale in the tree,
but it was stale before this branch and its drift is upstream Session Replay
API, not ours to carry in here.
Session Replay is not supported on native yet, so a replay rate the console
could set had nothing to act on here. Only the session sample rate is
delivered; the replay rate stays where the app configures it.

It is removed from the stored values, from the draw record events carry, and
from the bus message RUM sends Session Replay on renewal, so Session Replay
draws with exactly the sampler the app was built with. The forced-session flag
on that message is unaffected.
Web and iOS hand the application a dictionary; returning the raw JSON string
here made the same console value cost an extra parser on Android alone. Nested
objects and arrays come back as Map and List, and a body that cannot be read
answers as nothing published — no rate or decision depends on this bag.

Storage keeps the raw JSON, which has no reason to understand it.
`RemoteConfigValues.isEmpty()` had no caller anywhere: it meant something while
several knobs were delivered, and says nothing now that only the session sample
rate is. `readRate` took the field name from its single call site, and the retry
schedule was visible outside the file that is its only reader.
Reading a configuration response had two ways to go wrong, and neither
was handled.

A body that is not JSON threw out of the fetch. The parse ran inside a
try that only catches IOException and IllegalStateException, so a
JSONException escaped the whole method: the in-flight flag was never
cleared, every later trigger returned early, and remote configuration
stopped for the lifetime of the process with no log and no retry. A
captive portal answering 200 with a login page is enough to cause it,
and nothing checks the content type.

A body written to a newer contract was read field by field and applied.
The server states the shape it wrote in `schema_version`; a reader that
guesses instead of checking is exactly what that field exists to
prevent, and only code already on the device can refuse - a check added
in a later SDK would be ignored by the very clients it needs to protect.

`apply()` now reports one of three outcomes instead of throwing:

  APPLIED             the body was read and its values are stored
  UNREADABLE          not a configuration at all - ask again
  UNSUPPORTED_SCHEMA  a contract this SDK does not read - refused whole

Only UNREADABLE is retried. A schema we do not know is an answer, not a
failure: asking again would fetch the same refusal, so a server-side
schema bump cannot turn a fleet into a retry storm. Nothing from a
refused body reaches storage, not even the fields that happened to
parse, and the values already in use keep applying either way.
`setBeforeSampling` is consulted synchronously every time a new session
is about to be drawn, with the rate that would apply and the console's
custom values. Return a rate to override it, or null to leave it alone.
It runs after the console's rate on purpose: an allow-list is only
useful if it can keep collecting a visitor the console's rate would
drop.

Anything unusable - a null, a rate outside 0..100, a throw - leaves the
incoming rate alone. A mistake in the host application must never take a
customer's collection down with it.

Two behaviours are corrected to match the iOS and HarmonyOS SDKs, so one
console setting means one thing everywhere:

- setForcedSession() no longer restarts a session that is already being
  collected. RUM cannot retro-collect what a running session already
  dropped, so cutting it in two gained nothing; only an uncollected
  session is now replaced.
- rc_version is omitted rather than sent as 0 before the first
  configuration arrives, which is the shape the other platforms send.

Imports touched by the remote-configuration work are also sorted to the
layout .editorconfig declares.
The configuration a session was drawn under is held in memory and travels to
the view scopes that report it, which is all it is for. It was also written to
shared preferences on every session renewal, and nothing ever read it back: a
session does not survive the process here, so there is nothing for a stored
record to be restored into. What is left is a disk write per renewal and a JSON
codec kept alive to serve it.

The record itself, and everything that reports from it, is unchanged.
A response with no schema stamp at all was refused as a shape this SDK cannot
read, because the absent-value sentinel was compared against the supported
version like any other number. A body without a stamp is, by construction, the
shape that existed before the stamp did — which is the shape this reader was
written against. Refusing it switches remote configuration silently off against
a server that merely predates the field, and nothing says so: the refusal takes
the same path as a body we genuinely cannot read, so there is no error to
notice.

Only a stamp that is present and unrecognised is a refusal now, which is what
the web SDK already did. The two no longer disagree about the same response.
Removing the record's persistence left two of its three fields with no reader
at all: the rate the draw used travels down the scope chain as sampleRate and
is what every event already reports, and the session id was only ever the
validity check for the storage that is gone. Keeping them would be two records
of one fact, and one record of nothing.

The stamp check is also made strict. optInt would quietly turn the string "1"
into 1 and accept a body that iOS and HarmonyOS refuse, and a field whose whole
purpose is that every reader agrees about the same response cannot be the one
place they disagree. A stamp that is present but not a number is refused; an
explicit null reads as no stamp at all, which is what the other two do.
…nger runs

The store key covers the app version, so every release a device installs
leaves an entry behind, and nothing ever read or removed the one the
release before it used. They accumulated for good inside a preferences
file that is parsed in full at every launch.

Every write now stamps when it happened, and the first fetch of a launch
removes the entries nothing has refreshed for two days. Age is the only
thing that can separate an abandoned entry from a live one, and the
threshold has to clear the longest a live entry can legitimately stay
silent: the longest session, after which a new one fetches again, plus
the longest outage worth surviving, since a failed fetch stores nothing.

An unchanged answer comes back as a 304 with no body, which was the one
way to reach an entry and store nothing. A settled client meets it at
almost every fetch, so its entry now has its age refreshed there too.
Without that, two SDK instances in one app would each decide the other's
settled entry was abandoned and delete it at every launch.

This store's own entry is never a candidate, whatever its age says: it is
certainly in use, and on a first launch it has no write time at all.

The sweep runs on the worker thread, before anything is stored, and once
per launch — repeating it would walk the preferences file again at every
session start to learn nothing new. Releasing the in-flight flag moves
into a finally while it does: every later fetch is gated on that flag, so
anything that got out of fetchOnce without clearing it would have ended
remote configuration for the rest of the process, silently.
…renewal

A renewal replaces the scopes of the views that are still open, and the session
scope writes the new draw onto the view manager just before asking for that. The
manager was not handing those two values down: `renew()` built the replacement
from the ending view's own `sampleRate` and left `drawnConfiguration` at its
default of null.

That view is the first view of the new session, and the intake takes a session's
sample rate and configuration version from its first view and never restates
them, so the whole session was recorded under the rate of the session that had
just ended and under no configuration version at all. Weighted counts came out
wrong by the ratio between the two rates, and a session drawn under a newly
published configuration was not counted as having reached it.

It happens wherever a view is still open when a session renews: an inactivity or
maximum-duration renewal in the foreground, a configuration published for
immediate activation, and a session forced by the host application.

`renew()` now takes the draw it is renewing into, and the manager passes the
values it was just given. The existing test asserted that a renewed view kept the
old scope's rate, which is the behaviour above; it now passes a rate and a
configuration that differ from the ending view's and asserts the new one takes
them.
`setForcedSession()` promised that the forced state lasts for the process
lifetime and that the current session is restarted. Neither was true, and a
forced session also reported itself in a way no other SDK does.

The flag lived on the session scope, so `stopSession()` - the ordinary thing to
do on logout - left it behind: the next interaction built a fresh session scope
that was drawn as if the application had never asked. It now lives on the
application scope, which outlives any one session and hands it to every session
it makes. The documentation's other claim is corrected instead: a session already
being collected keeps running, because RUM cannot recover what a session already
dropped.

Events from a forced session reported the rate it would have been drawn at and
the configuration version in force. It was not drawn, so it now reports the rate
that describes it - every session like it is kept - and no version. Reporting the
drawn rate had the intake weight one deliberately kept session as the whole
population that rate implies, and left nothing to tell it apart from a lucky
draw.

Two events are also no longer acted on where doing so achieves nothing. A reset
of a forced session would only replace it with an identical forced session, at
the cost of the view the user is on, so it is ignored - which is the path a
configuration published for immediate activation takes. A reset or a force that
reaches a stopped, draining session scope would mint a session id under a scope
whose own context already reports the session as inactive, and announce that
session to the host application's listener, so both are ignored there too.

Adds coverage for the receiving half of forced Session Replay, which had none: a
renewal message carrying the forced flag records regardless of the replay
sampler, and one without it - the shape of every message written before the flag
existed - still does not.
…nfiguration

Four things the fetch path got wrong.

The ttl and the permission to refresh on returning to the foreground were held in
memory and set only when a body was read. An unchanged answer is a 304 with no
body, which is the steady state the validator exists to produce, so every launch
after the first ran on the defaults and the console's permission was silently
forgotten. They are now stored beside the values and read back before every
request, which also covers a launch whose first request fails. They are stored
while the configuration is switched off too, because the server goes on saying
when to ask again and a client that stopped honouring that would never learn it
had been switched back on.

The foreground counter had no floor. An app that initialises the SDK from an
activity - the usual shape when initialisation waits on a consent prompt - has
one running before the callback is registered, so the first stop it saw took the
count negative and it could never reach the one that means the app came back. The
class documentation also claimed rotation could not be mistaken for a return to
the foreground; it can, and that is now stated rather than denied.

An unstamped body was accepted. The schema stamp is the whole of what tells a
configuration apart from any other JSON, because every other field is read with a
default: an unrelated body - a proxy's block page, a reverse proxy answering
/config with something else - came out as "switched off, no rates", which is a
legitimate configuration, and storing it emptied the entry and dropped the client
back to the rates it was built with. An unstamped body is now treated as a
request that did not arrive: nothing is stored and it is asked again for. A stamp
that is present and unrecognised is still refused without a retry, since that is
an answer.

Failures were logged only to a target that reaches logcat in debug builds of the
SDK, so a device that quietly stopped taking the console's values said nothing to
anyone; they now go to telemetry as well. Opening the store also caught only
SecurityException, letting the IllegalStateException a direct-boot-aware
component gets before the device is unlocked escape into initialisation. The two
hand-written guards against a rejected submission are replaced by the core's own
safe submission helpers, which log and never throw.

Adds the coverage those paths lacked: a server error is not an answer and is
retried, an unstamped body is refused and retried, and a process whose first
answer is a 304 still refreshes on foreground when the store says it may - with
the negative control that one which stored nothing does not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant